gh-129069: Fix listobject.c data races due to memmove#142957
Merged
Conversation
The use of memmove and _Py_memory_repeat were not thread-safe in the free threading build in some cases. In theory, memmove and _Py_memory_repeat can copy byte-by-byte instead of pointer-by-pointer, so concurrent readers could see uninitialized data or tearing. Additionally, we should be using "release" (or stronger) ordering to be compliant with the C11 memory model when copying objects within a list.
colesbury
added
the
🔨 test-with-refleak-buildbots
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 97e8d13 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F142957%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
Yhg1s
approved these changes
Dec 19, 2025
Comment on lines
+1094
to
+1100
| PyObject **src = items; | ||
| PyObject **dst = items + input_size; | ||
| Py_ssize_t remaining = output_size - input_size; | ||
| while (remaining > 0) { | ||
| _Py_atomic_store_ptr_release(dst++, *src++); | ||
| remaining--; | ||
| } |
Member
There was a problem hiding this comment.
Why can't this just call list_atomic_memmove?
Contributor
Author
There was a problem hiding this comment.
Yeah, that will simplify this a bit and avoid the special case here (since list_atomic_memmove has it internally).
| goto Error; | ||
| } | ||
| Py_ssize_t tail = Py_SIZE(a) - ihigh; | ||
| list_atomic_memmove(a, &item[ihigh+d], &item[ihigh], tail); |
Member
There was a problem hiding this comment.
This is so much nicer, I wish we'd refactored this the first time round :P
| // Pointer-by-pointer memmove for PyObject** arrays that is safe | ||
| // for shared lists in Py_GIL_DISABLED builds. | ||
| static void | ||
| list_atomic_memmove(PyListObject *a, PyObject **dest, PyObject **src, Py_ssize_t n) |
Member
There was a problem hiding this comment.
Not a fan of the name (it's not the memmove as a whole that's atomic, just the individual writes) but I can't think of a better name and I think the comment explains it well enough.
Contributor
Author
There was a problem hiding this comment.
Yeah, it's a bit misleading. I can name it ptr_wise_atomic_memmove (as in Byte-wise atomic memcpy)
cocolato
pushed a commit
to cocolato/cpython
that referenced
this pull request
Dec 22, 2025
…-142957) The use of memmove and _Py_memory_repeat were not thread-safe in the free threading build in some cases. In theory, memmove and _Py_memory_repeat can copy byte-by-byte instead of pointer-by-pointer, so concurrent readers could see uninitialized data or tearing. Additionally, we should be using "release" (or stronger) ordering to be compliant with the C11 memory model when copying objects within a list.
|
GH-153791 is a backport of this pull request to the 3.14 branch. |
nascheme
added a commit
that referenced
this pull request
Jul 17, 2026
…132814) (GH-135689) (GH-144402) (GH-146033) (GH-142957) (GH-153791) Combined backport of PRs from main branch, fixing free-threading data-races in itertools: * gh-123471: make concurrent iteration over `itertools.cycle` safe under free-threading (gh-131212) * gh-123471: Make itertools.product and itertools.combinations thread-safe (GH-132814) * gh-123471: Make itertools.chain thread-safe (gh-135689) * gh-123471: Make concurrent iteration over `itertools.permutations` and `itertools.combinations_with_replacement` thread-safe (gh-144402) * gh-123471: make concurrent iteration over itertools.accumulate thread-safe (gh-144486) * gh-123471: Make `itertools.zip_longest` safe in the FT build (gh-146033) (cherry picked from commit 26a1cd4) (cherry picked from commit 847d1c2) (cherry picked from commit 0533c1f) (cherry picked from commit 009c8c0) (cherry picked from commit 3a24856) (cherry picked from commit 9214e3f) Co-authored-by: Pieter Eendebak <[email protected]> Co-authored-by: Kumar Aditya <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The use of memmove and _Py_memory_repeat were not thread-safe in the free threading build in some cases. In theory, memmove and _Py_memory_repeat can copy byte-by-byte instead of pointer-by-pointer, so concurrent readers could see uninitialized data or tearing.
Additionally, we should be using "release" (or stronger) ordering to be compliant with the C11 memory model when copying objects within a list.
I've also updated
list_resizeso it definitively doesn't fail when shrinking a list. This simplifies some of the call sites.